Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support fo "Renhållningen Kristianstad" in Kristianstad, Sweden #2741

Merged
merged 22 commits into from
Oct 1, 2024

Conversation

hansbacklund42
Copy link
Contributor

Added support for "Renhållningen Kristianstad" in Sweden to HACS waste collection service

Copy link
Collaborator

@5ila5 5ila5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you name the source krab_se? We typically name the source after the website ULR so renhallningen_kristianstad_se here.

Did you do any testing before submitting this? The source is fundamentally broken at multiple points. Please test your source using the test script (test_sources.py) before submitting a PR

@hansbacklund42
Copy link
Contributor Author

First of all naming conventions and naming.
I used that name because the company providing the service of collecting waste is KRAB and in Sweden.
It's an abreviation of "Kristianstad Renhållning AB" or as a weird translation it would be something like "Kristianstad Wastecollection Stock-Company" (or just Company, Ltd).
Depending on the definition of service provider the service provider it maybe should have been more like "api_universal_appbolaget_se" but that's just a url to a REST service on a consultant company and doesn't really define this waste collection.
I looked swiftly at some other implementations there are not a very strict naming convention as some refer to a more generic domain and some takes the name from a minicipality site not specific to waste collection.
But the name can of course be changed if that is critical.

I'm sorry for the code as it's really so many things that's wrong there. There's no other way of sayng it than it's totally FU.
It's a weird mix of what's running on my Home Assistant and some early mockup making it even worse than the mockup.
I'll be back with a working code.

@5ila5 5ila5 merged commit 22874af into mampfes:master Oct 1, 2024
2 checks passed
@5ila5
Copy link
Collaborator

5ila5 commented Oct 1, 2024

Thanks for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants